Spell perPage the same way in every paginated tool - #3142
Conversation
|
Can you make sure all schema advertised per_page entries are handled, I think there might be some others, and we should be totally consistent to reduce chances of bugs like this occuring. |
The schema declared per_page while the handler reads pagination through OptionalPaginationParams, which looks for perPage, so the value was always dropped and perPage fell back to the default 30.
projects_list advertised per_page while every other paginated tool advertises perPage. The handlers read per_page, so it worked, but it left one tool spelling pagination differently from the other 30 and that is how actions_list ended up advertising a name nothing read. The schema now says perPage. per_page is still read when perPage is absent: the projects tools have advertised it since September 2025 and clients sending it get the size they ask for today. TestAllToolInputSchemasUseCanonicalPaginationNames walks the whole tool inventory and rejects case and underscore variants of page, perPage, after and before. On main it fails on actions_list and projects_list.
efc5778 to
2c4b2a3
Compare
karpovantonme
changed the title
|
Went through the whole inventory rather than grepping for the string: 32 of the 117 tools advertise pagination, 30 of them get it from For the "reduce chances of bugs like this occurring" part I put the check next to your combinator guard in The |
2 participants
Summary
actions_listadvertisedper_pagewhile the handler reads pagination throughOptionalPaginationParams, which looks forperPage, so whatever the client sent was dropped and the size fell back to 30.projects_listadvertisesper_pagetoo and its handlers do read that name, so it works, but it left one tool spelling pagination differently from the other 30 and that is the gap the actions_list bug slipped through.Why
Both schemas now say
perPage, and a test keeps the rest of the inventory from drifting the same way.I went over the full inventory rather than grepping for one string: 32 of the 117 tools advertise pagination, 30 of them get it from
WithPagination/WithCursorPagination/WithUnifiedPagination, and these two are the only places where the properties are written out by hand.after_idandbefore_idon the sub-issue tools are positions rather than pagination, so the test leaves them be.What changed
pkg/github/actions.go:per_page->perPagein theactions_listschemapkg/github/projects.go: same in theprojects_listschema, and the three handlers read it through one helper.per_pageis still read whenperPageis absent, since the projects tools have advertised that name since September 2025 and clients sending it get the size they ask for todaypkg/github/tools_validation_test.go:TestAllToolInputSchemasUseCanonicalPaginationNameswalksAllToolsand rejects case and underscore variants ofpage,perPage,afterandbefore. On main it fails on exactly these two toolspkg/github/projects_test.go: covers the helper --perPage, legacyper_page, both together, neitherUPDATE_TOOLSNAPS=true, README withscript/generate-docsThe
per_pagefallback in projects is the one judgement call in here. Drop those three lines if you would rather make it a clean break, the test stays green either wayMCP impact
Two properties renamed.
actions_listclients sendingper_pagewere already being ignored, andprojects_listclients sending it keep working through the fallback.Prompts tested (tool changes only)
perPage: 1comes back with 1 run andper_page: 1with 30, byte for byte the same response as sending no page size at allprojects_listover stdio, no project at hand.Test_optionalProjectsPerPagecovers the four cases insteadSecurity / limits
Tool renaming
Lint & tests
./script/lint./script/testDocs